Simplified Bytes (internal)
#1099
Conversation
|
@jhorstmann I had to require |
Codecov Report
@@ Coverage Diff @@
## main #1099 +/- ##
==========================================
+ Coverage 81.23% 81.25% +0.02%
==========================================
Files 367 366 -1
Lines 35357 35336 -21
==========================================
- Hits 28721 28713 -8
+ Misses 6636 6623 -13
Continue to review full report at Codecov.
|
|
So we remove 1 layer of indirection to the data? And only decide at P.S. which Arc are you referring to? |
I believe so - although it is unclear whether the compiler was not optimizing it already.
The one at |
I don't think it can. It could be either branch, so it has to check. I think.
Right, and if we remove it, the clone call goes over FFI go that smart atomic pointer? |
| // This line is technically outside the assumptions of `Vec::from_raw_parts`, since | ||
| // `ptr` was not allocated by `Vec`. However, one of the invariants of this struct | ||
| // is that we do not expose this region as a `Vec`; we only use `Vec` on it to provide | ||
| // immutable access to the region (via `Vec::deref` to `&[T]`). | ||
| // MIRI does not complain, which seems to agree with the line of thought. |
There was a problem hiding this comment.
Maybe we could also use Box<[T]> and call Vec::from_raw_parts only on get_vec on native allocated data.
There was a problem hiding this comment.
I tried this but in get_vec we would be creating a temporary Vec and returning &mut Vec, which does not compile :(
jorgecarleitao
force-pushed
the
simpler_bytes
branch
from
5eb1d7d to
0220531
Compare
We do not know about smart pointers in FFI - in this case the idea is that we can |
This PR decouples
Bytesfrom everything exceptstd.This ports @jhorstmann's PR in arrow-rs. Although not as relevant here since we use
Vecanyways, it enable us to potentially moveBytesto its own crate.I am not fan of using
Arcinstead ofBox- the struct itself does not need anArc, will try to address this in a future PR.Relevant thread in rust-users: https://users.rust-lang.org/t/implementing-a-potentially-foreign-allocated-potentially-mutable-memory-region/77388